Make the CI lint job pass again #3691

DoctorJohn · 2024-11-07T16:21:52Z

Description

This PR makes the linter job pass again. A few weeks ago it started complaining about parts of the pydantic integration and codegen modules. I ignored it for too long and therefore didn't notice that my last PR introduced two minor issues. Time to fix it.

The first commit fixes variable redefinitions introduced in my last PR.
The other two commits aren't my proudest work but the best we can do short term to get CI pass again.

Types of Changes

Summary by Sourcery

Fix variable redefinitions and update type annotations to resolve mypy complaints and ensure the CI linter job passes.

Bug Fixes:

Fix variable redefinitions in the codebase to resolve issues introduced in the previous PR.

Enhancements:

Update type annotations and assertions to improve compatibility with mypy and ensure the linter job passes.

sourcery-ai · 2024-11-07T16:21:57Z

Reviewer's Guide by Sourcery

This PR addresses mypy type checking issues across multiple modules. The changes include fixing variable redefinitions in the GraphQL WebSocket handlers, improving type annotations in the codegen system, clarifying variable names in the channels testing module, and adding type ignores for pydantic integration fields.

Updated class diagram for codegen plugins

classDiagram
    class QueryCodegenPlugin {
    }
    class ConsolePlugin {
    }
    class Codegen {
        +_load_plugin(plugin_path: str) : Union[Type[QueryCodegenPlugin], Type[ConsolePlugin]]
        +_load_plugins(plugin_ids: List[str], query: Path) : List[Union[QueryCodegenPlugin, ConsolePlugin]]
    }
    Codegen --> QueryCodegenPlugin
    Codegen --> ConsolePlugin
    note for Codegen "Updated return types for _load_plugin and _load_plugins methods"

File-Level Changes

Change	Details	Files
Improved WebSocket message handling and type definitions in the GraphQL WS protocol	Refactored complete message creation to use CompleteMessage constructor Improved code organization with better spacing Eliminated variable redefinition for complete_message	`strawberry/subscriptions/protocols/graphql_ws/handlers.py`
Enhanced type annotations in the codegen system	Added Union type for plugin return types to include ConsolePlugin Updated function signatures with more specific return types Added type casting for plugins list Removed redundant query assignment to console plugin	`strawberry/cli/commands/codegen.py`
Improved variable naming in channels testing module	Renamed response variables to be protocol-specific Added more descriptive variable names for different protocol responses	`strawberry/channels/testing.py`
Added type ignores for pydantic field types	Added type: ignore comments for field type assignments Applied consistent type handling for both regular and extra fields	`strawberry/experimental/pydantic/object_type.py`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time. You can also use
this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @DoctorJohn - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

strawberry/subscriptions/protocols/graphql_ws/handlers.py

strawberry/cli/commands/codegen.py

botberry · 2024-11-07T16:24:19Z

Hi, thanks for contributing to Strawberry 🍓!

We noticed that this PR is missing a RELEASE.md file. We use that to automatically do releases here on GitHub and, most importantly, to PyPI!

So as soon as this PR is merged, a release will be made 🚀.

Here's an example of RELEASE.md:

Release type: patch

Description of the changes, ideally with some examples, if adding a new feature.

Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :)

Here's the tweet text:

🆕 Release (next) is out! Thanks to @NucleonJohn for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

codspeed-hq · 2024-11-07T16:31:47Z

CodSpeed Performance Report

Merging #3691 will not alter performance

_{Comparing DoctorJohn:make-linter-pass (07b286a) with main (184bf28)}

Summary

✅ 15 untouched benchmarks

patrick91

❤️

codecov · 2024-11-07T22:23:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.55%. Comparing base (184bf28) to head (07b286a).
Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (184bf28) and HEAD (07b286a). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (184bf28) HEAD (07b286a)

2 1

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3691       +/-   ##
===========================================
- Coverage   97.00%   62.55%   -34.46%     
===========================================
  Files         502      498        -4     
  Lines       33570    32411     -1159     
  Branches     5634     1670     -3964     
===========================================
- Hits        32566    20275    -12291     
- Misses        797    11687    +10890     
- Partials      207      449      +242

DoctorJohn added 3 commits November 7, 2024 17:06

Fix variable redefinitions

07e7165

Ignore trust issues

e4b6889

Narrow types of dynamically loaded plugins

07b286a

sourcery-ai bot reviewed Nov 7, 2024

View reviewed changes

strawberry/subscriptions/protocols/graphql_ws/handlers.py Show resolved Hide resolved

strawberry/cli/commands/codegen.py Show resolved Hide resolved

DoctorJohn changed the title ~~Make mypy happier and CI pass again~~ Make the CI lint job pass again Nov 7, 2024

patrick91 approved these changes Nov 7, 2024

View reviewed changes

DoctorJohn merged commit 04936fd into strawberry-graphql:main Nov 7, 2024
104 of 107 checks passed

DoctorJohn deleted the make-linter-pass branch November 20, 2024 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the CI lint job pass again #3691

Make the CI lint job pass again #3691

DoctorJohn commented Nov 7, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Nov 7, 2024 •

edited

Loading

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

botberry commented Nov 7, 2024

codspeed-hq bot commented Nov 7, 2024

patrick91 left a comment

codecov bot commented Nov 7, 2024

Make the CI lint job pass again #3691

Make the CI lint job pass again #3691

Conversation

DoctorJohn commented Nov 7, 2024 • edited by sourcery-ai bot Loading

Description

Types of Changes

Summary by Sourcery

sourcery-ai bot commented Nov 7, 2024 • edited Loading

Reviewer's Guide by Sourcery

Updated class diagram for codegen plugins

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

sourcery-ai bot left a comment

Choose a reason for hiding this comment

botberry commented Nov 7, 2024

codspeed-hq bot commented Nov 7, 2024

CodSpeed Performance Report

Merging #3691 will not alter performance

Summary

patrick91 left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 7, 2024

Codecov Report

DoctorJohn commented Nov 7, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Nov 7, 2024 •

edited

Loading